tls: disallow conflicting TLS protocol options#27521
tls: disallow conflicting TLS protocol options#27521sam-github wants to merge 1 commit intonodejs:masterfrom
Conversation
|
@nodejs/crypto |
|
Shouldn't |
sam-github
force-pushed
the
cli-tls-impossible
branch
2 times, most recently
from
927ad79 to
c6533c3
Compare
Do not allow the minimum protocol level to be set higher than the max protocol level. See: nodejs#26951, 109c097
sam-github
force-pushed
the
cli-tls-impossible
branch
from
c6533c3 to
918c0ed
Compare
sam-github
changed the title
|
Reworked this to disallow the option combination, PTAL @bnoordhuis @cjihrig |
| errors->push_back("invalid value for --unhandled-rejections"); | ||
| } | ||
|
|
||
| if (tls_min_v1_3 && tls_max_v1_2) { |
There was a problem hiding this comment.
node --tls-min-v1.3 --tls-max-v1.2 --tls-max-v1.3 currently does the right thing (sets min and max to 1.3) but this change makes it an error.
I'm okay with that, just pointing it out in case it's something we want to preserve. Changing the logic to tls_min_v1_3 && tls_max_v1_2 && !tls_max_v1_3 would do that.
There was a problem hiding this comment.
I would keep the check as it is. Having multiple flags that partially contradict each other could confuse people.
| errors->push_back("invalid value for --unhandled-rejections"); | ||
| } | ||
|
|
||
| if (tls_min_v1_3 && tls_max_v1_2) { |
There was a problem hiding this comment.
I would keep the check as it is. Having multiple flags that partially contradict each other could confuse people.
|
Still LGTM |
Trott
left a comment
Trott
left a comment
There was a problem hiding this comment.
LGTM. (Theoretically, throwing an error where there wasn't one before is generally semver-major but I don't think that logic applies here.)
|
Landed in cb848b4 |
Do not allow the minimum protocol level to be set higher than the max protocol level. See: #26951, 109c097 PR-URL: #27521 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com>
Do not allow the minimum protocol level to be set higher than the max protocol level. See: #26951, 109c097 PR-URL: #27521 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Rich Trott <rtrott@gmail.com>
6 participants
Do not allow the minimum protocol level to be set higher than the max protocol level.
See: #26951, 109c097
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes